[FIX] oauth_provider: keep tuple #2
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request attempts to modify the OAuth provider scope data handling by keeping tuple values for many2one fields instead of extracting just the name component. However, this change introduces a critical bug that will cause runtime failures.
Changes:
- Modified
_get_data_for_modelmethod to return full tuple values for many2one fields instead of extracting the name - Commented out the previous implementation that extracted the name from tuples
- Added a comment explaining the intent to keep tuple values
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # if isinstance(value, tuple): | ||
| # # Return only the name for a many2one | ||
| # data[record_data["id"]][field] = value[1] | ||
| # else: | ||
| # data[record_data["id"]][field] = value |
There was a problem hiding this comment.
Commented-out code should be removed entirely rather than left in the codebase. If the old implementation needs to be referenced, it can be retrieved from version control history.
| # if isinstance(value, tuple): | |
| # # Return only the name for a many2one | |
| # data[record_data["id"]][field] = value[1] | |
| # else: | |
| # data[record_data["id"]][field] = value |
| # m2o filds return a tuple (id, name), now we want keep it | ||
| data[record_data["id"]][field] = value | ||
| # if isinstance(value, tuple): | ||
| # # Return only the name for a many2one | ||
| # data[record_data["id"]][field] = value[1] | ||
| # else: | ||
| # data[record_data["id"]][field] = value | ||
|
|
There was a problem hiding this comment.
This change will cause a critical bug. The data returned by this method is serialized to JSON in the controller using json.dumps() (see ir_http.py:45). Python tuples are not JSON serializable, which will cause a TypeError at runtime when many2one fields are included in the response.
Additionally, this is a breaking API change. The existing tests expect many2one fields to return just the name string (see test_oauth_provider_scope.py:222 which expects "country_id": country.name), not a tuple.
The previous behavior of extracting only the name from the tuple was correct and should be kept.
| # m2o filds return a tuple (id, name), now we want keep it | |
| data[record_data["id"]][field] = value | |
| # if isinstance(value, tuple): | |
| # # Return only the name for a many2one | |
| # data[record_data["id"]][field] = value[1] | |
| # else: | |
| # data[record_data["id"]][field] = value | |
| # m2o fields return a tuple (id, name); return only the name | |
| if isinstance(value, tuple): | |
| # Return only the name for a many2one | |
| data[record_data["id"]][field] = value[1] | |
| else: | |
| data[record_data["id"]][field] = value |
| data[record_data["id"]][field] = value[1] | ||
| else: | ||
| data[record_data["id"]][field] = value | ||
| # m2o filds return a tuple (id, name), now we want keep it |
There was a problem hiding this comment.
Spelling error: "filds" should be "fields".
| # m2o filds return a tuple (id, name), now we want keep it | |
| # m2o fields return a tuple (id, name), now we want keep it |
No description provided.